Skip to content

Rename EagerThunk to DTask #518

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
May 23, 2024
Merged

Rename EagerThunk to DTask #518

merged 5 commits into from
May 23, 2024

Conversation

jpsamaroo
Copy link
Member

@jpsamaroo jpsamaroo commented May 22, 2024 •
edited
Loading

EagerThunk is a terrible name - let's change this to something a bit more easily explainable. Also adds a better docstring for Dagger.@spawn to help explain how it differs from Julia's Task.

Also deprecates delayed - this API is no longer recommended for various reasons, and will be removed as a user-facing API in the future.

@jpsamaroo jpsamaroo requested review from pszufe and JamesWrigley May 22, 2024 16:04
JamesWrigley
JamesWrigley reviewed May 22, 2024
View reviewed changes
Copy link
Collaborator

@JamesWrigley JamesWrigley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big fan of both of these changes :) I think we can go ahead and bump the version number already since this will be breaking. Another thing I noticed is that a lot of the mentioned symbols in the docstrings could be converted into linked references with e.g. [`Dagger.@spawn()`](@ref), but I can fix those later.

@JamesWrigley
Copy link
Collaborator

The array allocation test failure on 1.10 is a little concerning 🤔 Is this the first time it happened or is that just a flaky test?

@jpsamaroo
Copy link
Member Author

The array allocation test failure on 1.10 is a little concerning 🤔 Is this the first time it happened or is that just a flaky test?

Just a flaky test! I guess it's possible to get 0 from rand() - I should probably test that there are less than 5% of zeros or something like that.

@jpsamaroo jpsamaroo merged commit 66ba6a9 into master May 23, 2024
8 of 12 checks passed
@jpsamaroo jpsamaroo deleted the jps/dtask branch May 23, 2024 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JamesWrigley JamesWrigley JamesWrigley left review comments

@pszufe pszufe Awaiting requested review from pszufe

Assignees
No one assigned
Labels
documentation eager api
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jpsamaroo @JamesWrigley